fix(metadata): properly handle casting non-numeric values in search operations with custom metadata #55184
Conversation
tcitworld
requested review from
icewind1991 and
provokateurin
and removed request for
a team
|
/backport to stable32 |
|
/backport to stable31 |
…perations with custom metadata
If a metadata property is declared with a number type but the value provided are not numeric, it
logs "A non-numeric value encountered at nextcloud/apps/dav/lib/Files/FileSearchBackend.php#486" instead of throwing a proper error.
Now with a proper error we have the proper exception being thrown: InvalidArgumentException Invalid
property value for {http://nextcloud.org/ns}metadata-photos-original_date_time
Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
force-pushed
the
handle-metadata-dav-search-error
branch
from
96beacb to
974cfef
Compare
| if (is_numeric($value)) { | ||
| return 0 + $value; | ||
| } | ||
| throw new \Error('Value for numeric datatype is not numeric'); |
There was a problem hiding this comment.
Not sure if throwing in error is better or simply ignoring it with: return 0;
There was a problem hiding this comment.
My reasoning was indeed that it allows for the throw new \InvalidArgumentException('Invalid property value for ' . $property->name, previous: $e); exception to be thrown, so that the front-end can know something's wrong. Otherwise I might not have found about nextcloud/photos#3187
There was a problem hiding this comment.
I tried this:
throw new \InvalidArgumentException(
sprintf(
'Invalid property value for %s: got "%s" (type: %s)',
$property->name,
is_scalar($value) ? (string)$value : gettype($value),
gettype($value)
)
);and got 2 errors:
InvalidArgumentException Invalid property value for {http://owncloud.org/ns}size: got "0" (type: string)
InvalidArgumentException Invalid property value for {http://nextcloud.org/ns}metadata-photos-original_date_time: got "2024-10-09T00:00:00Z" (type: string)
I'm not sure if these errors can be safely ignored. If these warnings are not important, I'd prefer they not show up in the log.
4 participants
If a metadata property is declared with a number type but the value provided are not numeric, it just logs "A non-numeric value encountered at
apps/dav/lib/Files/FileSearchBackend.php#486" instead of throwing a proper error.Now with a proper error here we have the proper exception being thrown later:
InvalidArgumentException Invalid property value for {http://nextcloud.org/ns}metadata-photos-original_date_time.See nextcloud/photos#3187 for the actual issue in photos.
(I'm too lazy to add a specific test for this, but feel free to)
Checklist